Introduce HKDF to PKCS11 TA by Hussainity · Pull Request #7837 · OP-TEE/optee_os

Hussainity · 2026-06-09T15:54:04Z

This PR implements CKM_HKDF_DERIVE (PKCS#11 v3.0, RFC 5869) in the PKCS#11 TA so that clients can perform HKDF Extract, Expand, or Extract-then-Expand against a generic-secret parent key.

Summary

New ta/pkcs11/src/processing_hkdf.c: HKDF Extract / Expand built directly on TEE_ALG_HMAC_* rather than the GP TEE_ALG_HKDF so all three modes (extract-only, expand-only, both) and any HMAC-supported PRF hash (MD5, SHA-1, SHA-224/256/384/512) can be supported. Short keys are zero-padded to the GP HMAC type minimum (equivalent under RFC 2104) so RFC 5869 test vectors with sub-minimum salts work.
Mechanism plumbing:
- ta/pkcs11/include/pkcs11_ta.h: add PKCS11_CKM_HKDF_DERIVE and the pkcs11_hkdf_salt_type enum mirroring CKF_HKDF_SALT_{NULL,DATA,KEY}.
- ta/pkcs11/src/pkcs11_attributes.c: allow the new mechanism through created-attribute and parent-key checks (parent must be CKO_SECRET_KEY / CKK_GENERIC_SECRET).
- ta/pkcs11/src/processing.{c,h}: dispatch PKCS11_FUNCTION_DERIVE to derive_key_by_hkdf().
- ta/pkcs11/src/token_capabilities.c: advertise the mechanism with CKFM_DERIVE.
Wire format matches libckteec's serialize_mecha_hkdf_derive_param().
Unrelated supporting fix: core/tee/tee_svc_cryp.c and lib/libutee/tee_api_operations.c — TEE_ALG_HKDF (consolidated form, no hash bits in the algo ID) now defaults to SHA-256 in get_hkdf_params() and is accepted by TEE_AllocateOperation(). Without this, an operation for TEE_ALG_HKDF returns TEE_ERROR_NOT_SUPPORTED.

RFC 5869 conformance

Extract: PRK = HMAC-Hash(salt, IKM); empty salt → HashLen zero bytes.
Expand: T(i) = HMAC-Hash(PRK, T(i-1) || info || i), N = ceil(L / HashLen) ≤ 255.
Mode-specific length checks: extract-only output must equal HashLen; expand-only requires IKM ≥ HashLen; expand caps L ≤ 255 · HashLen.

Related PRs

OPTEE Client: libckteec: add CKM_HKDF_DERIVE mechanism support optee_client#420
OPTEE Test: xtest: include tests for HKDF_DERIVE optee_test#819

Testing

xtest ran on QEMUv8

jenswikl · 2026-06-10T07:39:21Z

Please provide a proper commit message.

etienne-lms · 2026-06-22T17:29:43Z

 	PKCS11_CKM_SHA512_RSA_PKCS_PSS		= 0x00045,
 	PKCS11_CKM_SHA224_RSA_PKCS		= 0x00046,
 	PKCS11_CKM_SHA224_RSA_PKCS_PSS		= 0x00047,
+	PKCS11_CKM_HKDF_DERIVE			= 0x0402a,


I'd prefer we keep on sorting enum labels per value. Could you move PKCS11_CKM_HKDF_DERIVE down to the end of the enum list?

etienne-lms · 2026-06-22T17:34:43Z

 	PKCS11_CKM_UNDEFINED_ID			= PKCS11_UNDEFINED_ID,
 };

+/* HKDF salt source selectors (mirror libckteec CKF_HKDF_SALT_*) */


Suggestion, for consistency with other inlie descriptions

Suggested change

/* HKDF salt source selectors (mirror libckteec CKF_HKDF_SALT_*) */

/*

* Valid values for HKDF source selectors

* PKCS11_CKF_HKDF_SALT_<x> reflects CryptoKi client API IDs CKF_HKDF_SALT_<x>.

*/

etienne-lms · 2026-06-22T17:35:40Z

+		if (key_class != PKCS11_CKO_SECRET_KEY ||
+		    key_type != PKCS11_CKK_GENERIC_SECRET)
+			return PKCS11_CKR_KEY_FUNCTION_NOT_PERMITTED;
+		break;


Could you add an empty line below, for conssitency.

etienne-lms · 2026-06-22T17:46:33Z

+	} else if (processing_is_tee_hkdf(proc_params->id)) {
+		if (function != PKCS11_FUNCTION_DERIVE) {
+			rc = PKCS11_CKR_MECHANISM_INVALID;
+			goto out;


If function != PKCS11_FUNCTION_DERIVE I think we should not have reached this point.
TEE_Panic(function) could apply.
IMHO ASSERT(function != PKCS11_FUNCTION_DERIVE) could be enough.

I picked TEE_Panic(function) for consistency

etienne-lms · 2026-06-22T17:50:17Z

+}
+
+/* Mirrors the strict GP HMAC type ranges from core/tee/tee_svc_cryp.c */
+struct hkdf_prf {


Prefer have struct defined before any fucntion: here, before processing_is_tee_hkdf() definition.

etienne-lms · 2026-06-22T18:57:56Z

 	TA_MECHANISM(PKCS11_CKM_AES_ECB_ENCRYPT_DATA, PKCS11_CKFM_DERIVE),
 	TA_MECHANISM(PKCS11_CKM_AES_CBC_ENCRYPT_DATA, PKCS11_CKFM_DERIVE),
 	TA_MECHANISM(PKCS11_CKM_ECDH1_DERIVE, PKCS11_CKFM_DERIVE),
+	TA_MECHANISM(PKCS11_CKM_HKDF_DERIVE, PKCS11_CKFM_DERIVE),


Prefer adding this line at the end of the array, for consistency with pkcs11_modes[].

etienne-lms · 2026-06-22T19:03:07Z

+		if (rc)
+			return rc;
+	} else {
+		*salt = NULL;


Could remove *salt = NULL (also *info = NULL below) since we known these are already zero initialiaze.

etienne-lms · 2026-06-22T19:06:15Z

+		salt = NULL;
+		salt_len = 0;


IMO it would be worth be strict on expected parameters provided by the client:

Suggested change

salt = NULL;

salt_len = 0;

if (salt || salt_len)

rc = PKCS11_CKR_MECHANISM_PARAM_INVALID;

etienne-lms · 2026-06-22T19:24:37Z

+		if (!salt_obj) {
+			rc = PKCS11_CKR_KEY_HANDLE_INVALID;
+			goto out;
+		}


Should check access to salt key handle.

rc = check_access_attrs_against_token(session, salt_obj->attributes); if (rc) goto out;

etienne-lms · 2026-06-22T19:31:45Z

+		res = TEE_ERROR_GENERIC;
+
+out:
+	memset(padded_key, 0, sizeof(padded_key));


memzero_explicit()

etienne-lms · 2026-06-23T07:14:56Z

I forgot to thank you for these additions. I'll need to take a few other looks, for the details, but the overall LGTM.

Hussainity · 2026-06-26T20:45:41Z

I forgot to thank you for these additions. I'll need to take a few other looks, for the details, but the overall LGTM.

@etienne-lms , happy to contribute, thank you for the review, I have updated the branch.

github-actions · 2026-07-27T03:58:31Z

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

etienne-lms · 2026-07-27T07:19:18Z

Keep alive.

etienne-lms

For commit "core: tee: tee_svc_cryp: default TEE_ALG_HKDF hash to SHA-256" and
commit "libutee: tee_api_operations: accept TEE_ALG_HKDF in TEE_AllocateOperation":
Look consistent to me.
Reviewed-by: Etienne Carriere <etienne.carriere@st.com>

For commit "ta: pkcs11: Introduce HKDF_DERIVE support", some minor comments. The overall looks good to me but I need another review.

My apologies for this late feedback on your P-R. Thanks for your patience.

etienne-lms · 2026-07-27T16:24:49Z

+		if (function != PKCS11_FUNCTION_DERIVE) {
+			TEE_Panic(function);
+		}


For consistency, prefer without braces for single line conditioned instructions:

Suggested change

if (function != PKCS11_FUNCTION_DERIVE) {

TEE_Panic(function);

}

if (function != PKCS11_FUNCTION_DERIVE)

TEE_Panic(function);

etienne-lms · 2026-07-27T16:30:13Z

+		memzero_explicit(okm, out_byte_size);
+		TEE_Free(okm);
+	}
+	release_active_processing(session);


release_active_processing() is already called from entry_processing_key(). For consistency I'd prefer you remove this line.

etienne-lms · 2026-07-27T16:44:16Z

+		if (!rc)
+			goto done;
+		goto out;


For consistency with the other cases in this function, prefer the other way round:

Suggested change

if (!rc)

goto done;

goto out;

if (rc)

goto out;

goto done;

etienne-lms · 2026-07-27T16:47:36Z

+	if (key_obj != TEE_HANDLE_NULL)
+		TEE_FreeTransientObject(key_obj);
+	if (op != TEE_HANDLE_NULL)
+		TEE_FreeOperation(op);


Could remove the tests:

Suggested change

if (key_obj != TEE_HANDLE_NULL)

TEE_FreeTransientObject(key_obj);

if (op != TEE_HANDLE_NULL)

TEE_FreeOperation(op);

TEE_FreeTransientObject(key_obj);

TEE_FreeOperation(op);

etienne-lms · 2026-07-27T16:53:16Z

+	case PKCS11_CKF_HKDF_SALT_KEY: {
+		struct pkcs11_object *salt_obj = NULL;
+		void *salt_value = NULL;
+		uint32_t salt_value_size = 0;


The braces added here make indentation a bit strange. I'd prefer you move the local variable at function entry or use a local helper function for the salt key case.

Hussainity · 2026-07-30T14:25:48Z

@etienne-lms , I appreciate your review, I have updated the branch with your suggestions.

etienne-lms

Still LGTM. See the remaining comments.

etienne-lms · 2026-07-30T15:36:12Z

+	rc = get_u32_attribute(*head, PKCS11_CKA_VALUE_LEN, &out_byte_size);
+	if (rc)
+		goto out;


The specs says generated object template may not specify CKA_VALUE_LEN when bExpand is false, in which case the length is defined by the algo.
Suggestion:

Suggested change

rc = get_u32_attribute(*head, PKCS11_CKA_VALUE_LEN, &out_byte_size);

if (rc)

goto out;

rc = get_u32_attribute(*head, PKCS11_CKA_VALUE_LEN, &out_byte_size);

if (rc) {

/* When bExpand is false, default default lengh is the hash length */

if (rc != PKCS11_RV_NOT_FOUND)

goto out;

if (expand == PKCS11_TRUE) {

rc = PKCS11_CKR_MECHANISM_PARAM_INVALID;

goto out;

}

out_byte_size = prf.hash_len;

}

etienne-lms · 2026-07-30T15:48:45Z

+	if (rc)
+		goto out;
+
+	switch (salt_type) {


Nitpicking: thee spec says the slat should be ignored when bExtract is false. I wonder if a strict implementation should avoid error cases when salt parameters are invalid, like skipping this switch/case sequence when so. What do you think?

Agreed, always supportive of removing vendor specific failure modes that aren't reflected in the spec!

Hussainity · 2026-08-09T21:44:16Z

@etienne-lms , addressed comments and tested against OP-TEE/optee_test#819

Hussainity · 2026-08-12T15:47:00Z

Will add review tags for all commits once I get the thumbs up

Hussainity · 2026-08-18T20:31:46Z

@etienne-lms @jenswikl I fixed the workflow errors, still need a maintainer rerun and reviewer tag for the final commit!

jforissier · 2026-08-20T09:49:41Z

For "core: tee: tee_svc_cryp: default TEE_ALG_HKDF hash to SHA-256" and "libutee: tee_api_operations: accept TEE_ALG_HKDF in TEE_AllocateOperation": Reviewed-by: Jerome Forissier <jerome.forissier@arm.com>

The consolidated TEE_ALG_HKDF carries no hash bits in its algorithm ID. tee_cryp_hkdf() runs the value through TEE_ALG_HASH_ALGO(), so seed hash_id with TEE_ALG_GET_DIGEST_HASH(TEE_ALG_HKDF_SHA256_DERIVE_KEY) rather than TEE_ALG_SHA256 to match that form. Signed-off-by: Hussain Miyaziwala <hussain_miya@hotmail.com> Reviewed-by: Etienne Carriere <etienne.carriere@st.com> Reviewed-by: Jerome Forissier <jerome.forissier@arm.com>

…tion The consolidated TEE_ALG_HKDF identifier was missing from the switch in TEE_AllocateOperation(), so allocating an operation for it returned TEE_ERROR_NOT_SUPPORTED. Add it alongside the per-hash HKDF variants. Signed-off-by: Hussain Miyaziwala <hussain_miya@hotmail.com> Reviewed-by: Etienne Carriere <etienne.carriere@st.com> Reviewed-by: Jerome Forissier <jerome.forissier@arm.com>

Add the PKCS#11 v3.0 CKM_HKDF_DERIVE mechanism. The implementation runs HKDF (RFC 5869) directly on top of TEE_ALG_HMAC_* so that extract-only, expand-only and combined modes are all supported, for any HMAC PRF the GP API offers (MD5, SHA-1, SHA-224/256/384/512). The mechanism parameters follow the wire format produced by libckteec (bExtract, bExpand, prfHashMechanism, salt source/length/data, hSaltKey, info length/data) and the parent key must be a CKO_SECRET_KEY of CKK_GENERIC_SECRET that supplies the IKM. Signed-off-by: Hussain Miyaziwala <hussain_miya@hotmail.com>

Hussainity mentioned this pull request Jun 9, 2026

RFC: Add HKDF Key Derivation to the PKCS11 TA #7825

Closed

Hussainity force-pushed the hm/hkdf-001 branch from f9c5c32 to 8398120 Compare June 10, 2026 17:00

etienne-lms reviewed Jun 22, 2026

View reviewed changes

Hussainity force-pushed the hm/hkdf-001 branch from 8398120 to 13d6ede Compare June 26, 2026 20:42

github-actions Bot added the Stale label Jul 27, 2026

etienne-lms reviewed Jul 27, 2026

View reviewed changes

github-actions Bot removed the Stale label Jul 28, 2026

Hussainity force-pushed the hm/hkdf-001 branch from 13d6ede to 666ef0d Compare July 30, 2026 14:24

etienne-lms reviewed Jul 30, 2026

View reviewed changes

Hussainity force-pushed the hm/hkdf-001 branch from 666ef0d to 132a173 Compare August 9, 2026 21:39

Hussainity force-pushed the hm/hkdf-001 branch from 132a173 to 09a85c2 Compare August 11, 2026 15:20

Hussainity added 3 commits August 31, 2026 14:22

Hussainity force-pushed the hm/hkdf-001 branch from 09a85c2 to 2e52a12 Compare August 31, 2026 18:25

-/* HKDF salt source selectors (mirror libckteec CKF_HKDF_SALT_*) */
+/*
+ * Valid values for HKDF source selectors
+ * PKCS11_CKF_HKDF_SALT_<x> reflects CryptoKi client API IDs CKF_HKDF_SALT_<x>.
+ */

-	rc = get_u32_attribute(*head, PKCS11_CKA_VALUE_LEN, &out_byte_size);
-	if (rc)
-		goto out;
+	rc = get_u32_attribute(*head, PKCS11_CKA_VALUE_LEN, &out_byte_size);
+	if (rc) {
+		/* When bExpand is false, default default lengh is the hash length */
+		if (rc != PKCS11_RV_NOT_FOUND)
+			goto out;
+		if (expand == PKCS11_TRUE) {
+			rc = PKCS11_CKR_MECHANISM_PARAM_INVALID;
+			goto out;
+		}
+		out_byte_size = prf.hash_len;
+	}

Uh oh!

Conversation

Hussainity commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

RFC 5869 conformance

Related PRs

Testing

Uh oh!

jenswikl commented Jun 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etienne-lms commented Jun 23, 2026

Uh oh!

Hussainity commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jul 27, 2026

Uh oh!

etienne-lms commented Jul 27, 2026

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hussainity commented Jul 30, 2026

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hussainity commented Aug 9, 2026

Uh oh!

Hussainity commented Aug 12, 2026

Uh oh!

Hussainity commented Aug 18, 2026

Uh oh!

jforissier commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Hussainity commented Jun 9, 2026 •

edited

Loading